Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MIWI cluster Dynamic Validation update for strict 1:1 matching for provided Platform Workload Identity to expected OCP Operators #3966

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

MIWI cluster Dynamic Validation update for strict 1:1 matching for provided Platform Workload Identity to expected OCP Operators #3966

wants to merge 6 commits into from
+340 −186

Conversation

rajdeepc2792
Copy link
Collaborator

@rajdeepc2792 rajdeepc2792 commented Nov 20, 2024 •
edited
Loading

Which issue this PR addresses:

Fixes ARO-10859

What this PR does / why we need it:

  • Static validation at RP/frontend when platformworkloadidentity profile is nil or map is empty.
  • Stop using platformWorkloadIdentityRolesByVersion for the delete flow so that dependency on the roleset for the OCP version can be removed.
  • Fails create/update flow whenever an unexpected Platform Workload Identity is found.
  • Update dynamic validation for strictly matching the Platform Workload Identity's operator names with expected OCP Operators.(Previously additional identities were allowed)

Test plan for issue:

[x] Unit tests were added/updated for the above implementation
[x] Create/Update MIWI cluster in local
[x] CI
[x] e2e

Is there any documentation that needs to be updated for this PR?

Not yet.

How do you know this will function as expected in production?

Feature is not in production yet.

@rajdeepc2792 rajdeepc2792 self-assigned this Nov 20, 2024
@rajdeepc2792 rajdeepc2792 added hold Hold chainsaw Pull requests or issues owned by Team Chainsaw labels Nov 20, 2024
@rajdeepc2792 rajdeepc2792 force-pushed the rajdeepc2792/ARO-10859 branch 3 times, most recently from 744b3ad to da1decc Compare November 22, 2024 13:39
@github-actions github-actions bot added the needs-rebase branch needs a rebase label Nov 22, 2024
@github-actions GitHub Actions
Copy link

Please rebase pull request.

@rajdeepc2792 rajdeepc2792 marked this pull request as ready for review November 22, 2024 15:23
@github-actions github-actions bot removed the needs-rebase branch needs a rebase label Nov 22, 2024
@rajdeepc2792 rajdeepc2792 mentioned this pull request Nov 25, 2024
Draft
@rajdeepc2792 rajdeepc2792 force-pushed the rajdeepc2792/ARO-10859 branch 2 times, most recently from 8fb81f7 to ec9136e Compare November 26, 2024 13:06
kimorris27
kimorris27 requested changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@kimorris27 kimorris27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except for one tiny suggestion on a cx-facing error message.

pkg/api/v20240812preview/openshiftcluster_validatestatic.go Outdated Show resolved Hide resolved
@kimorris27
Copy link
Contributor

/azp run e2e

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kimorris27
Copy link
Contributor

/azp run ci

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kimorris27
Copy link
Contributor

E2E failed with a known flake that's already captured in our CI failure epic. I'll wait to run it again until Rajdeep responds to my most recent review.

@rajdeepc2792
Copy link
Collaborator Author

/azp run ci

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jewzaam jewzaam Awaiting requested review from jewzaam jewzaam is a code owner

@bennerv bennerv Awaiting requested review from bennerv bennerv is a code owner

@hawkowl hawkowl Awaiting requested review from hawkowl hawkowl is a code owner

@rogbas rogbas Awaiting requested review from rogbas rogbas is a code owner

@petrkotas petrkotas Awaiting requested review from petrkotas petrkotas is a code owner

@jharrington22 jharrington22 Awaiting requested review from jharrington22 jharrington22 is a code owner

@cblecker cblecker Awaiting requested review from cblecker cblecker is a code owner

@cadenmarchese cadenmarchese Awaiting requested review from cadenmarchese cadenmarchese is a code owner

@UlrichSchlueter UlrichSchlueter Awaiting requested review from UlrichSchlueter UlrichSchlueter is a code owner

@SudoBrendan SudoBrendan Awaiting requested review from SudoBrendan SudoBrendan is a code owner

@yjst2012 yjst2012 Awaiting requested review from yjst2012 yjst2012 is a code owner

@jaitaiwan jaitaiwan Awaiting requested review from jaitaiwan jaitaiwan is a code owner

@anshulvermapatel anshulvermapatel Awaiting requested review from anshulvermapatel anshulvermapatel is a code owner

@hlipsig hlipsig Awaiting requested review from hlipsig hlipsig is a code owner

@tiguelu tiguelu Awaiting requested review from tiguelu tiguelu is a code owner

@SrinivasAtmakuri SrinivasAtmakuri Awaiting requested review from SrinivasAtmakuri

@mociarain mociarain Awaiting requested review from mociarain mociarain is a code owner

@tsatam tsatam Awaiting requested review from tsatam tsatam is a code owner

@bitoku bitoku Awaiting requested review from bitoku bitoku is a code owner

@fahlmant fahlmant Awaiting requested review from fahlmant fahlmant is a code owner

@kimorris27 kimorris27 Awaiting requested review from kimorris27 kimorris27 is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

@rajdeepc2792 rajdeepc2792

Labels
chainsaw Pull requests or issues owned by Team Chainsaw ready-for-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rajdeepc2792 @kimorris27